Take in middleware factory rather than middleware#270
Conversation
|
not necessary :) thanks for the 2nd pair of eyes on this, @amory-coursera |
|
Reviving this - while middlewares are indeed stateless, reusing the same middleware for every request seemed to have resulted in a memory leak due to the thread-local scope data not getting cleaned up. Here's the heap dump for the newest deploy that resulted in java heap shooting up rapidly: https://imgur.com/a/t5SH0CV (~11GB heap) Datadog link of deploy to ~40min after deploy: https://app.datadoghq.com/dashboard/a4f-p8g-3mx/service-dash?from_ts=1561553831162&live=true&tile_size=m&to_ts=1561568231162&tpl_var_service=assembler&fullscreen_widget=58099565&fullscreen_section=overview Here's the heap dump for the last known good deploy (which still has tracing enabled): Datadog link of deploy to ~40min after deploy: https://app.datadoghq.com/dashboard/a4f-p8g-3mx/service-dash?from_ts=1561553831162&live=true&tile_size=m&to_ts=1561568231162&tpl_var_service=assembler&fullscreen_widget=58099565&fullscreen_section=overview My best guess is because we create a singleton tracing middleware, every thread-local created has a strong reference that's never cleaned up at the end of the request. EDIT: also added datadog links to correlate |
|
Gentle ping on this - would like to resolve by end of week if at all possible. Thanks in advance! |
amory-coursera
left a comment
There was a problem hiding this comment.
This feels like something we could fix in another way, but if this is the path of least resistance I'm ok with the approach.
I think the existence of thread-locals in middlewares along with async execution makes all other solutions very non-obvious. Assuming this fix indeed does the job, I think it is the path of least resistance and the most obviously correct solution. |
PTAL @dguo-coursera, @amory-coursera - I think middlewares should be created per request rather than once on instantiation, as they hold execution state. This along with another change should unbreak the datadog tracing.